psql/migrations: when no-transaction is set, split multi-statement sql into individual statements and execute them separately #3574
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is going to be a controversial one.
Recently, support for adding
-- no-transaction
at the beginning of migration files was introduced, which will execute the migration without creating a transaction. This is useful for certain commands, such asCREATE INDEX CONCURRENTLY
, which are not allowed to run inside transactions.However, postgres will implicitly create a transaction block when a query contains multiple statements. This means that the
-- no-transaction
flag has inconsistent behavior depending on whether the migration contains just one statement or multiple ones, and that commands that are not allowed to run inside transactions will fail if part of a larger migration.I propose to split the sql into individual statements when
no_tx
is set and to execute them separately, resulting in consistent behavior in line with how many sql tools handle this case. This was implemented using a custompest
grammar, which is very low overhead and performant (but does add a dependency).One could argue that the current behavior is good (albeit lacking a clear error message) and that users should be encouraged to create small migrations with only one statement at a time when opting out of the safety of transactions. I would counter that a user that opts out of transactions usually has a reason for that and is aware that extra care has to be taken to ensure that migration is sound and tested, that an error during a
no_tx
migration will always require manual cleanup anyway, and that there are production cases where forcing the user to split up a logical migration into potentially many individual files can add complexity and make it harder to manage and audit (we recently had such case).